Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Monomers ports #1

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Monomers ports #1

wants to merge 7 commits into from

Conversation

blairlyons
Copy link
Collaborator

Problem

We're working on visualizing a simple ReaDDy model (simularium/vivarium-models#19) in Simularium as it calculates live, and we need updates to the ports structure and emitter in order to convert data for SImularium in the same way we visualize actin models from ReaDDy.

Solution

Updated the ports structure to match other ReaDDy processes, made implementation more general wrt scale and system, and updated the emitter accordingly.

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Change summary:

  • use the ports schema for monomers from our models collab repo (https://github.com/simularium/vivarium-models), so we can handle all data from ReaDDy similarly
  • add time and space units to support other scales, since ReaDDy can use any units as long as they are consistent. generalize diffusion coefficient calculation to match
  • only create the ReaDDy system at init and reuse it for each simulation step. Previously it was created every timestep. This is a huge gain in efficiency and the tradeoff is that all the particle species have to be declared at start
  • add visualization radius to particle state data so it doesn't need to be hardcoded in the emitter
  • some code clean up

Copy link
Member

@eagmon eagmon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor comments, everything looks great. Just a couple questions: is SimulariumEmitter no longer used anywhere? Is there a corresponding SimulariumFilamentEmitter to the SimulariumMonomerEmitter? I think we should aim for a general SimulariumEmitter, so that it can handle all possible cases -- if we use only emitters that are specialized for a single data type, the simulation won't be able to stream the other data types that might be available.

}
},
}
return monomer_ports_schema
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, I like this reuse of schema.

self.system.potentials.add_harmonic_repulsion(
type1,
type2,
force_constant=self.parameters["force_constant"],
interaction_distance=(
self.parameters["particle_radii"][type1]
+ self.parameters["particle_radii"][type2]
self.parameters["particle_radii"].get(type1, 1.0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should use a default size set in self.parameters instead of a hardcoded 1.0?

Comment on lines +1 to +4
import numpy as np


monomer_ports_schema = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would name this file schema.py instead of utils. It seems like a file that could be expanded upon with more schemas for reuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants